fix: Lighthouse a11y + CLS issues on homepage#431
Conversation
Addresses real failures surfaced after PR LLM-Coding#430 pointed Lighthouse at the canonical deployed URL instead of the stale fork URL. ## Accessibility (0.92 → target 1.0) ### aria-allowed-role + label-content-name-mismatch Anchor cards used <article role="button" aria-label="Open {title} details">, which fails two axe rules: - <article> has implicit role "article"; overriding to "button" is invalid per ARIA in HTML spec - aria-label started with "Open" instead of the visible h3 text, so the accessible name didn't match the visible label Fixed by changing <article> to <div role="button"> and using aria-labelledby pointing at the h3 id. Accessible name now exactly matches the visible title. Event delegation uses the .anchor-card class, not the tag name, so nothing breaks. ### select-name #header-role-filter had no associated label. Added aria-label referencing the existing "All roles" translation string. ### color-contrast .meta-badge text inherited .anchor-card-meta's gray-500/400 colors, which only reach ~3.9:1 (light) / ~3.1:1 (dark) on the badge's gray-100/700 background — below the WCAG AA 4.5:1 threshold for normal text. Explicitly set gray-700/200 on .meta-badge for ~5.3:1 in both themes. ## Performance (0.75 → target ≥ 0.9) ### cumulative-layout-shift (weight 25) #page-content started empty and populated asynchronously with the card grid, causing the single biggest CLS contributor on the page. Added min-height: calc(100vh - 12rem) to reserve viewport space so the browser doesn't shift layout when the content arrives. ### unsized-images Logo images (header + onboarding modal) had no explicit width/height, so the browser couldn't reserve space and had to reflow when the image loaded. Added width/height attributes derived from the actual logo.png aspect ratio (1024×451) and added w-auto so the CSS height constraint still wins visually. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 28 minutes and 47 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughDas PR aktualisiert Website-Komponenten mit verbesserten Barrierefreiheitsattributen und expliziten Bildabmessungen. Das includes Änderungen am Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
website/src/components/card-grid.js (1)
122-125:aria-labelledby-Ziel-ID sollte pro Karteninstanz eindeutig sein.Aktuell hängt die ID nur von
anchor.idab. Falls ein Anchor mehrfach gerendert wird, entstehen doppelte IDs im DOM.♻️ Vorschlag für eindeutige IDs
- ${categoryAnchors.map((anchor) => renderAnchorCard(anchor, color)).join('')} + ${categoryAnchors.map((anchor) => renderAnchorCard(anchor, color, category.id)).join('')} -function renderAnchorCard(anchor, categoryColor) { +function renderAnchorCard(anchor, categoryColor, categoryId) { const isUmbrella = anchor.subAnchors && anchor.subAnchors.length > 0 @@ const safeId = escapeHtml(anchor.id) + const safeCategoryId = escapeHtml(categoryId) + const cardTitleId = `anchor-card-title-${safeCategoryId}-${safeId}` return ` <div @@ - aria-labelledby="anchor-card-title-${safeId}" + aria-labelledby="${cardTitleId}" > <div class="anchor-card-header"> - <h3 id="anchor-card-title-${safeId}" class="anchor-card-title">${escapeHtml(anchor.title)}</h3> + <h3 id="${cardTitleId}" class="anchor-card-title">${escapeHtml(anchor.title)}</h3>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/components/card-grid.js` around lines 122 - 125, The aria-labelledby target must be unique per card instance: update the markup that uses safeId/anchor.id so the generated id for the heading and the aria-labelledby attribute includes an instance-unique suffix (e.g., a per-instance counter, timestamp or short UUID) and use that same composed id for both the aria-labelledby and the h3 id (symbols to change: safeId, "anchor-card-title-${...}", and the aria-labelledby attribute in the card render in card-grid.js) so duplicated anchors cannot produce duplicate DOM ids.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/src/components/header.js`:
- Around line 57-58: The mobile language toggle still uses a hardcoded
aria-label ("Toggle language"); update the mobile toggle element in header.js
(the mobile language toggle button) to use i18n.t('header.langToggleAria') for
its aria-label and add the same data-i18n-aria="header.langToggleAria" attribute
as the desktop toggle so the accessible name matches localized text. Locate the
mobile toggle button (the element with aria-label="Toggle language") and replace
the static string with the i18n lookup and add the data-i18n-aria attribute.
In `@website/src/translations/de.json`:
- Line 5: The de.json entry for "header.langToggleAria" currently mixes English
explanatory text; update the value so the visible language code remains "EN" but
the explanatory phrase is German (e.g., "EN — zu Englisch wechseln") so
screenreader and UI are fully localized; locate the "header.langToggleAria" key
in website/src/translations/de.json and replace the English phrase with the
German equivalent while keeping the "EN" token intact.
---
Nitpick comments:
In `@website/src/components/card-grid.js`:
- Around line 122-125: The aria-labelledby target must be unique per card
instance: update the markup that uses safeId/anchor.id so the generated id for
the heading and the aria-labelledby attribute includes an instance-unique suffix
(e.g., a per-instance counter, timestamp or short UUID) and use that same
composed id for both the aria-labelledby and the h3 id (symbols to change:
safeId, "anchor-card-title-${...}", and the aria-labelledby attribute in the
card render in card-grid.js) so duplicated anchors cannot produce duplicate DOM
ids.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b8c5b3b1-d8f5-42d2-bbaf-6bbcafc4f83e
📒 Files selected for processing (6)
website/src/components/card-grid.jswebsite/src/components/header.jswebsite/src/components/onboarding-modal.jswebsite/src/styles/main.csswebsite/src/translations/de.jsonwebsite/src/translations/en.json
1. Mobile lang-toggle still had hardcoded aria-label="Toggle language".
Now uses i18n.t('header.langToggleAria') + data-i18n-aria, matching
the desktop fix from the parent commit.
2. de.json aria-label was English ("EN — switch to English"). Now
properly localized: "EN — zu Englisch wechseln".
3. Anchor card heading ids are now namespaced with the category id.
An anchor can belong to multiple categories and is rendered once per
category section, so the previous "anchor-card-title-<id>" pattern
produced duplicate DOM ids for those anchors. New pattern:
"anchor-card-title-<categoryId>-<anchorId>".
Two follow-ups to LLM-Coding#433 against the persistent CLS 0.975 issue. ## 1. Logo: max-h-24 → h-24 (explicit height) Lighthouse continued to attribute the layout shift to the header logo even after LLM-Coding#431 added width="218" height="96" attributes. Root cause: Tailwind's preflight resets all images to `height: auto`, which overrides the HTML height attribute. `max-h-24` only caps the height, it doesn't enforce one — so before the image loads, the rendered height is `auto` (= 0). When the PNG actually loads, the header grows by 96px and pushes everything below. `h-24` (height: 6rem) explicitly sets the height in CSS, overriding preflight's `auto`. Combined with `w-[218px]` (and the matching mobile sizes), the browser reserves the exact box from first paint. ## 2. Shell-header / shell-footer dimensions match the rendered ones The static skeleton in LLM-Coding#433 used min-height: 10.5rem for the header placeholder, but the real <header> renders at ~8.75rem (140px: py-3 + h-24 logo + slogan + border). When hydrateShell() replaced the placeholder, the header shrank by ~28px and #page-content moved up by the same amount — a small but real shift on top of every other load contribution. Tighten the placeholders to match: header 8.75rem, footer 6rem (already correct), and update #page-content min-height accordingly. These are smaller contributions than the cards-loading shift, but they clean up the easily-fixable signal first so the next Lighthouse run isolates the remaining CLS to the actual hard problem. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Real issues surfaced after #430 pointed Lighthouse at the canonical URL. Current scores on
llm-coding.github.io/Semantic-Anchors/:The single failing perf audit with non-zero weight is Cumulative Layout Shift (weight 25, score 0.02). Accessibility has 4 failing audits. This PR addresses each root cause.
Accessibility fixes
aria-allowed-role<article role="button">— article has implicit role "article", overriding to "button" is invalid per ARIA in HTML<article>→<div role="button">label-content-name-mismatcharia-label="Open {title} details"doesn't start with the visible h3 textaria-labelledbypointing at the h3id, so the accessible name exactly matches the visible titleselect-name#header-role-filterhad no labelaria-labelwith existingfilter.allRolestranslationcolor-contrast.meta-badgeinherited gray-500/400 text on gray-100/700 bg → ~3.9:1 / ~3.1:1 (below WCAG AA 4.5:1)text-gray-700 dark:text-gray-200→ ~5.3:1 in both themeslabel-content-name-mismatch(lang-toggle)aria-label="Toggle language"— no overlapheader.langToggleAriastring that starts with the visible text: "DE — switch to German"Performance fixes
cumulative-layout-shift(weight 25)#page-contentempty on first paint, populated async with card gridmin-height: calc(100vh - 12rem)reserves viewport spaceunsized-imageswidth="218" height="96"(desktop) /width="145" height="64"(mobile) from actual 1024×451 aspect ratio, plusw-autoso CSS height constraint still winsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Verbesserungen für Barrierefreiheit
Stil
Bug-Behebung